fix(ui): hide only the top bar on tablet/desktop scroll, keep search#1238
Conversation
Anchor the Therapy secondary strip inside universal-header-collapse so it hides and reveals with the top bar on the shared scroll signal. Desktop keeps the in-flow sticky nav in the workspace.
Document the header-collapse addon rule and lock the phone portal wiring with static contracts plus a Chromium hide/reveal check.
The prior chrome-scroll suite only checked data-scroll-hidden on forms, so wrapping the search composer inside the collapse row could still go green. Assert header-band search geometry stays on screen, and register the Therapy nav scroll spec in the production Playwright matcher.
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughThe PR separates the collapsing top bar from the search composer, portals Therapy section navigation into the phone collapse host, updates responsive styling and routing, and expands contract, Playwright, smoke, and production test selection coverage. ChangesResponsive header collapse
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant MainContent
participant UniversalHeaderCollapse
participant TherapyCompassNav
User->>MainContent: scroll therapy results
MainContent->>UniversalHeaderCollapse: update data-scroll-hidden
UniversalHeaderCollapse->>TherapyCompassNav: hide or reveal collapse host
TherapyCompassNav-->>User: show section navigation when header returns
Possibly related PRs
Suggested labels: Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dashboard result composers still cleared a 4.75rem top-bar band after the mode bar hid, leaving a dead gap above search. Rise to the viewport top while collapse hide is active.
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #5757 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
Phone section nav portals outside .tc-root into the header collapse host, so closest(.tc-root) was empty. Read the workspace root from the document and assert the strip is anchored under universal-header-collapse.
Sticky-stack hosts already pin [top bar | search]; a second sticky composer with its own top offset overlays page controls once the top bar collapses. Keep that composer in normal flow inside the outer stack. Also target the decision-rail Clear by title and re-query Compare after RightRail remounts.
A bare ixed class on the composer survived beside sm:relative under Tailwind's cascade, so GlobalSearchShell search kept overlaying page controls (Services Clear/Compare). Scope fixed to max-sm when the outer sticky stack owns tablet/desktop placement, and assert Compare by title.
Keep sticky-stack composers in normal flow (relative), harden Services rail testids/scroll-into-view for the pinned search band, and accept sticky ancestors in desktop composer placement checks.
Document the Services-rail click hazard under the pinned sticky-stack search band and keep the header scroll contract aligned.
Keep top-bar-only collapse + sticky search stack while adopting main's always-on chrome-safe-area-top spacer and submitted-result focus rules.
Collapsed chrome now sits below chrome-safe-area-top, so the phone nav off-screen check must use the collapse host top rather than viewport y=0.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db4390b381
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // Phones: portal into the collapsing top-bar track so the strip hides/reveals | ||
| // with the universal header. sm+: keep in-flow sticky inside the workspace. | ||
| if (phoneHost) return createPortal(navShell, phoneHost); |
There was a problem hiding this comment.
On phone widths, this portals the section nav into a header host outside .tc-root, but its layout rules remain scoped as .tc-root .tc-nav-001, .tc-root .tc-nav-007, and .tc-root .tc-btn; the added portal-specific CSS only overrides position, top, and z-index. Consequently the phone nav loses its flex centering, padding/background, horizontal scrolling, and scoped focus/interaction styles, so the seven controls can wrap or overflow instead of remaining a compact scrollable strip. A focused browser check of the portaled nav's computed display, overflow-x, padding, and background at 390px would catch this; either preserve the .tc-root scope at the portal destination or duplicate the applicable scoped rules for the portal host.
Useful? React with 👍 / 👎.
Append-only ledger closeout after #1238 landed. RAG impact: no retrieval behaviour change — ledger docs only.
1 participant
Summary
Tablet/desktop hide-on-scroll was hiding the search field with the mode/new-chat top bar. Root cause: the collapse/translate wrapper wrapped
headerAndComposer(top bar + search). Phone docks escape viaposition: fixed; inline tablet/desktop search does not.This PR collapses only
header#search(plus optional Therapy phone nav addon). Search stays a sibling. Sticky hosts pin an outer[top bar | search]stack below the always-onchrome-safe-area-topspacer without translating the search away.Also fixed while landing CI
relativein the outer stack (no second sticky / no barefixedcascade fight that overlays Services rail controls).scrollIntoViewso Playwright does not park rail clicks under the pinned search band.position: stickyon the form.mainsafe-area spacer + submitted-result focus rules without restoring whole-stack translate hide.chrome-safe-area-top.Verification
npx vitest run tests/header-scroll-hide-contract.test.ts(14/14)ui-chrome-scroll(12/12) on pre-merge tip; Therapy nav rechecked after safe-area merge12a21258(288+ tests; earlier tip)fdc20bfe(Therapy nav assert fix)RAG impact
RAG impact: no retrieval behaviour change — UI chrome / search placement only.
Risk
UI chrome only. Watch tablet/desktop: top bar hides, search stays, status-bar safe-area never releases, Services rail remains clickable under the sticky search band.
Summary by CodeRabbit
New Features
Bug Fixes
Tests